Replace connect with useSelector() and useDispatch() 2/5 #2314 #2333

ahtesham-quraish · 2025-07-28T10:00:46Z

Description

Replace connect with useSelector() and useDispatch() 2/5 #2314

Supporting information

Link to other information about the change, such as GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Please provide detailed step-by-step instructions for manually testing this change.

Other information

Include anything else that will help reviewers and consumers understand the change.

Does this change depend on other changes elsewhere?
Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.

Best Practices Checklist

We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:

Any new files are using TypeScript (.ts, .tsx).
Deprecated propTypes, defaultProps, and injectIntl patterns are not used in any new or modified code.
Tests should use the helpers in src/testUtils.tsx (specifically initializeMocks)
Do not add new fields to the Redux state/store. Use React Context to share state among multiple components.
Use React Query to load data from REST APIs. See any apiHooks.ts in this repo for examples.
All new i18n messages in messages.ts files have a description for translators to use.
Imports avoid using ../. To import from parent folders, use @src, e.g. import { initializeMocks } from '@src/testUtils'; instead of from '../../../../testUtils'

…enedx#231

codecov · 2025-07-29T08:25:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.45%. Comparing base (0e1550a) to head (eb476c8).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2333   +/-   ##
=======================================
  Coverage   94.44%   94.45%           
=======================================
  Files        1169     1169           
  Lines       25102    25130   +28     
  Branches     5473     5490   +17     
=======================================
+ Hits        23708    23736   +28     
+ Misses       1328     1322    -6     
- Partials       66       72    +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Faraz32123 · 2025-07-29T11:36:44Z

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.test.tsx

+  // const showAdvancedSettingsCardsBaseProps = {
+  //   isAdvancedCardsVisible: false,
+  //   showAdvancedCards: jest.fn(),
+  //   setResetTrue: jest.fn(),
+  // };


Can we remove this or any unnecessary comments?

bradenmacdonald

Thanks for this! It's a nice refactor and I'm excited to have this in place. However, could you please make sure to review #2343 and rebase this on top of it, before proceeding?

bradenmacdonald · 2025-07-30T20:44:20Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

  beforeEach(() => {
-    initializeMocks();
+    initializeMocks({
+      initialState,


Passing initialState twice (to initializeMocks and editorRender) is redundant - please see the fixes in #2343

I have removed the initialState from initializeMocks

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.jsx

bradenmacdonald · 2025-07-30T20:49:14Z

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.test.tsx

@@ -17,6 +17,49 @@ jest.mock('./settingsComponents/SwitchEditorCard', () => 'SwitchEditorCard');
 jest.mock('./settingsComponents/TimerCard', () => 'TimerCard');
 jest.mock('./settingsComponents/TypeCard', () => 'TypeCard');

+jest.mock('../../../../../data/redux', () => ({


Please declare this using the EditorState type (or PartialEditorState from #2343) so we know that the types are valid. I suspect there are some issues here.

I have used the PartialEditorState from #2343 you can check that

src/editors/modifiedEditorTestRender.jsx

ahtesham-quraish · 2025-08-04T13:23:44Z

@bradenmacdonald thank you for highlighting the typed issues I have tried to fix them in my current PR and will fix the remaining ones in follow-up PRs. I have gone through the #2343 and approved it, after its merge I will rebase my branch

bradenmacdonald · 2025-08-05T23:06:11Z

Thanks @ahtesham-quraish. I've merged it now so you can rebase this branch.

ahtesham-quraish · 2025-08-06T10:17:00Z

@bradenmacdonald I have rebased my branch with #2343 please re-review my PR

bradenmacdonald

Looks great! Thanks @ahtesham-quraish. I have one question about preloadedState being {} vs undefined which I'd like to know before merging. The rest is minor/optional.

bradenmacdonald · 2025-08-15T01:01:41Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

-  },
-}));
+import { screen, initializeMocks } from '@src/testUtils';
+import { editorRender } from '../../../../../editorTestRender';


Suggested change

import { editorRender } from '../../../../../editorTestRender';

import { editorRender, type PartialEditorState } from '@src/editors/editorTestRender';

bradenmacdonald · 2025-08-15T01:05:32Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

@@ -36,19 +16,24 @@ jest.mock('../../../../../sharedComponents/TinyMceWidget', () => ({
  default: () => <div>TinyMceWidget</div>,
 }));

-describe('SolutionWidget', () => {
-  const props = {
+const initialState = {


Suggested change

const initialState = {

const initialState: PartialEditorState = {

bradenmacdonald · 2025-08-15T01:05:55Z

...editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.tsx

    settings: { solutionExplanation: 'This is my solution' },
+  },
+  app: {
    learningContextId: 'course+org+run',
    images: {},
    isLibrary: false,


Suggested change

isLibrary: false,

I know you didn't add this line, but once we put the types in you can see it needs to be removed.

bradenmacdonald · 2025-08-15T01:07:57Z

src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.test.tsx

-  },
-}));
+import { screen, initializeMocks } from '@src/testUtils';
+import { editorRender } from '../../../../../editorTestRender';


Suggested change

import { editorRender } from '../../../../../editorTestRender';

import { editorRender, type PartialEditorState } from '@src/editors/editorTestRender';

bradenmacdonald · 2025-08-15T01:08:05Z

src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.test.tsx

@@ -39,23 +13,25 @@ jest.mock('@src/editors/sharedComponents/TinyMceWidget/hooks', () => ({

 jest.mock('@src/editors/sharedComponents/TinyMceWidget', () => ('TinyMceWidget'));

-describe('QuestionWidget', () => {
-  const props = {
+const initialState = {


Suggested change

const initialState = {

const initialState: PartialEditorState = {

bradenmacdonald · 2025-08-15T01:10:15Z

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/index.tsx

@@ -131,36 +154,36 @@ const SettingsWidget = ({
          {!isLibrary && (
            <div className="my-3">
              <ResetCard
-                showResetButton={settings.showResetButton}
+                showResetButton={settings.showResetButton!}


Suggested change

showResetButton={settings.showResetButton!}

showResetButton={settings.showResetButton || false}

Nit: this would be nicer than using ! I think.

bradenmacdonald · 2025-08-15T01:11:37Z

src/editors/data/redux/index.ts

@@ -194,7 +194,7 @@ export interface EditorState {

 export { actions, selectors };

-export function initializeStore(preloadedState = undefined) {
+export function initializeStore(preloadedState = {}) {


Why is {} a better default than undefined ? I thought if we left this undefined it would use the "normal" initial state, whereas if we set it, it will use an empty initial state with no keys or values. But I could be wrong.

openedx-webhooks · 2025-08-15T01:18:54Z

Thanks for the pull request, @ahtesham-quraish!

This repository is currently maintained by @bradenmacdonald.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Submit a signed contributor agreement (CLA)

⚠️ We ask all contributors to the Open edX project to submit a signed contributor agreement or indicate their institutional affiliation.
Please see the CONTRIBUTING file for more information.

If you've signed an agreement in the past, you may need to re-sign.
See The New Home of the Open edX Codebase for details.

Once you've signed the CLA, please allow 1 business day for it to be processed.
After this time, you can re-run the CLA check by adding a comment below that you have signed it.
If the CLA check continues to fail, you can tag the @openedx/cla-problems team in a comment for further assistance.

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

Ahtesham Quraish and others added 2 commits July 24, 2025 22:08

refactor: Replace connect with useSelector() and useDispatch() 1/5

2bb2b76

refactor: Replace connect with useSelector() and useDispatch() 2/5 op…

cced750

…enedx#231

ahtesham-quraish requested a review from bradenmacdonald July 29, 2025 09:19

Faraz32123 reviewed Jul 29, 2025

View reviewed changes

ahtesham-quraish force-pushed the ahtesham/#2314 branch 2 times, most recently from 4a5c655 to 0cfc7f8 Compare July 30, 2025 09:47

bradenmacdonald requested changes Jul 30, 2025

View reviewed changes

ahtesham-quraish requested review from Faraz32123 and bradenmacdonald July 31, 2025 13:03

ahtesham-quraish force-pushed the ahtesham/#2314 branch 5 times, most recently from a9ab34f to a414746 Compare August 4, 2025 11:05

fix: merge with main branch

652bfff

ahtesham-quraish force-pushed the ahtesham/#2314 branch from a414746 to 652bfff Compare August 4, 2025 11:09

fix: rebase with master

eb476c8

ahtesham-quraish force-pushed the ahtesham/#2314 branch from 99a44e2 to eb476c8 Compare August 6, 2025 07:45

bradenmacdonald reviewed Aug 15, 2025

View reviewed changes

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 15, 2025

openedx-webhooks added this to Contributions Aug 15, 2025

github-project-automation bot moved this to Needs Triage in Contributions Aug 15, 2025

mphilbrick211 moved this from Needs Triage to Needs Tests Run or CLA Signed in Contributions Aug 21, 2025

mphilbrick211 moved this from Needs Tests Run or CLA Signed to In Eng Review in Contributions Aug 21, 2025

	import { editorRender } from '../../../../../editorTestRender';
	import { editorRender, type PartialEditorState } from '@src/editors/editorTestRender';

	const initialState = {
	const initialState: PartialEditorState = {

	showResetButton={settings.showResetButton!}
	showResetButton={settings.showResetButton \|\| false}

Replace connect with useSelector() and useDispatch() 2/5 #2314 #2333

Are you sure you want to change the base?

Replace connect with useSelector() and useDispatch() 2/5 #2314 #2333

Conversation

ahtesham-quraish commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Other information

Best Practices Checklist

Uh oh!

codecov bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahtesham-quraish commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradenmacdonald commented Aug 5, 2025

Uh oh!

ahtesham-quraish commented Aug 6, 2025

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Aug 15, 2025

Uh oh!

Uh oh!

ahtesham-quraish commented Jul 28, 2025 •

edited

Loading

codecov bot commented Jul 29, 2025 •

edited

Loading

ahtesham-quraish commented Aug 4, 2025 •

edited

Loading